Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added brave product urls to brave://about #9785

Merged
merged 1 commit into from
Aug 24, 2021
Merged

added brave product urls to brave://about #9785

merged 1 commit into from
Aug 24, 2021

Conversation

nullhook
Copy link
Contributor

@nullhook nullhook commented Aug 18, 2021 •
edited
Loading

Resolves brave/brave-browser#17299

image

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@nullhook nullhook requested review from a team as code owners August 18, 2021 22:03
@nullhook nullhook requested review from petemill and bsclifton August 18, 2021 22:05
@nullhook nullhook force-pushed the bug/brave-host-urls branch from 12521ad to 9e487de Compare August 18, 2021 22:28
@bsclifton
Copy link
Member

bsclifton commented Aug 19, 2021 •
edited
Loading

Not sure if it's due to Chromium 93 merging? or if it's Windows specific (unlikely) but this isn't working for me unfortunately

@goodov
Copy link
Member

goodov commented Aug 19, 2021

@bsclifton make sure the patch is applied

@bsclifton
Copy link
Member

@goodov good call! Total rookie mistake there 😂 (didn't put branch name in package.json when running init). OK looks like it's got a failed patch after 93 merged

@bsclifton bsclifton force-pushed the bug/brave-host-urls branch 3 times, most recently from faf4156 to b8833f0 Compare August 20, 2021 07:47
@bsclifton
Copy link
Member

Rebased and updated patch 😄

bsclifton
bsclifton approved these changes Aug 20, 2021
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@Tonev
Copy link
Contributor

Tonev commented Aug 21, 2021

Since this PR is related to brave://about, it might be a good idea to remove/fix brave://memories as it leads to nowhere currently.

Reported in Brave Community: https://community.brave.com/t/brave-memories-not-available/273961

@bsclifton
Copy link
Member

bsclifton commented Aug 23, 2021 •
edited
Loading

Thanks for following up on that @Tonev - and thanks in general for being super helpful with issue triage 😄 I definitely appreciate the help you've brought here (and on community.brave.com)

There is an issue tracking the removal of that - brave/brave-browser#15582

I'll leave it up to @nullhook if we want to solve in the same pull request 😄 (otherwise, I'm happy to fix)

@nullhook nullhook force-pushed the bug/brave-host-urls branch from b8833f0 to 6814d3e Compare August 23, 2021 18:38
@nullhook
Copy link
Contributor Author

Thanks the pointing the issue @Tonev - let's do this on another PR @bsclifton

@nullhook nullhook force-pushed the bug/brave-host-urls branch from 6814d3e to 5ac00cd Compare August 23, 2021 18:53
@nullhook nullhook force-pushed the bug/brave-host-urls branch from 5ac00cd to c742948 Compare August 23, 2021 22:03
@nullhook nullhook merged commit 3edae68 into master Aug 24, 2021
@nullhook nullhook deleted the bug/brave-host-urls branch August 24, 2021 03:07
@nullhook nullhook added this to the 1.30.x - Nightly milestone Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@goodov goodov goodov approved these changes

@petemill petemill Awaiting requested review from petemill

Assignees

@nullhook nullhook

Labels
bug UI
Projects
None yet
Milestone
1.30.x - Release
Development

Successfully merging this pull request may close these issues.

brave://about missing Brave-specific URLs
4 participants
@nullhook @bsclifton @goodov @Tonev